Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Data] Refactor some file-related unit tests #48228

Merged
merged 2 commits into from
Oct 23, 2024
Merged

[Data] Refactor some file-related unit tests #48228

merged 2 commits into from
Oct 23, 2024

Conversation

bveeramani
Copy link
Member

Why are these changes needed?

This PR refactors several unit tests to simplify them and make them less brittle (e.g., by parameterizing inputs or removing ordering assumptions).

Related issue number

Checks

  • I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
    • I've added any new APIs to the API Reference. For example, if I added a
      method in Tune, I've added it in doc/source/tune/api/ under the
      corresponding .rst file.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu>
Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu>
Comment on lines -776 to -778
error_message = "No input files found to read"
with pytest.raises(ValueError, match=error_message):
ray.data.read_csv(path3, file_extensions=["csv"]).schema()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't test that this raises a value error, because returning an empty dataset is also valid

@@ -846,7 +813,7 @@ def test_csv_read_with_column_type_specified(shutdown_only, tmp_path):
Version(pa.__version__) < Version("7.0.0"),
reason="invalid_row_handler was added in pyarrow 7.0.0",
)
def test_csv_invalid_file_handler(shutdown_only, tmp_path):
def test_csv_invalid_file_handler(ray_start_regular_shared, tmp_path):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we update other tests to use ray_start_regular_shared instead of shutdown_onl? e.g. test_csv_read_with_column_type_specified

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we should. Can do in a follow-up PR if/when I have time

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow-up here: #48352

@bveeramani bveeramani enabled auto-merge (squash) October 23, 2024 21:28
@github-actions github-actions bot added the go add ONLY when ready to merge, run all tests label Oct 23, 2024
@bveeramani bveeramani merged commit f8651c6 into master Oct 23, 2024
6 checks passed
@bveeramani bveeramani deleted the fix-diff branch October 23, 2024 22:19
bveeramani added a commit that referenced this pull request Oct 29, 2024
See
#48228 (comment).

Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu>
Jay-ju pushed a commit to Jay-ju/ray that referenced this pull request Nov 5, 2024
This PR refactors several unit tests to simplify them and make them less brittle (e.g., by parameterizing inputs or removing ordering assumptions).

Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu>
Jay-ju pushed a commit to Jay-ju/ray that referenced this pull request Nov 5, 2024
JP-sDEV pushed a commit to JP-sDEV/ray that referenced this pull request Nov 14, 2024
This PR refactors several unit tests to simplify them and make them less brittle (e.g., by parameterizing inputs or removing ordering assumptions).

Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu>
JP-sDEV pushed a commit to JP-sDEV/ray that referenced this pull request Nov 14, 2024
mohitjain2504 pushed a commit to mohitjain2504/ray that referenced this pull request Nov 15, 2024
This PR refactors several unit tests to simplify them and make them less brittle (e.g., by parameterizing inputs or removing ordering assumptions).

Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu>
Signed-off-by: mohitjain2504 <mohit.jain@dream11.com>
mohitjain2504 pushed a commit to mohitjain2504/ray that referenced this pull request Nov 15, 2024
…#48352)

See
ray-project#48228 (comment).

Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu>
Signed-off-by: mohitjain2504 <mohit.jain@dream11.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
go add ONLY when ready to merge, run all tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants